Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Upgrade ruby to 2.7 and jemalloc to v5 #4027

Merged
merged 6 commits into from
Nov 22, 2022
Merged

Conversation

camallen
Copy link
Contributor

This PR is related to the change in #4023 that switched to v2 bundler version

This is due to the CD system using our ruby2.6 docker image which doesn't have the v2 version of bundler available.

The bad news is our ruby 2.6 docker image is more than 1 year old and unsupported both in terms of the underlying OS debian version (stretch) and the ruby version.
https://www.ruby-lang.org/en/downloads/releases/
https://hub.docker.com/_/ruby/tags?page=1&name=2.6-slim-stretch

Sadly we can't rely on this image anymore and it's time to upgrade it.

The good news is we have an easy upgrade to ruby 2.7 though that too is on the way to EOL https://www.ruby-lang.org/en/news/2022/04/12/ruby-2-7-6-released/

The only tricky part here is the upgrade to 2.7 means we are now on debian bullseye which installs v5 of the jemalloc memory allocator. I've included a MALLOC_CONF env var config to make jemalloc v5 behave like v3 in terms of memory savings vs raw performance so we shouldn't see big changes in overall RAM use on our systems.

Once approved we can get this deployed to staging for testing and monitoring via new relic ruby vm memory use graphs to determine if we see unacceptable changes.

As some background - i looked into fullstaq ruby which comes with v3 jemalloc built into ruby via the docker images supplied by https://github.com/evilmartians/fullstaq-ruby-docker. However it appears that they believe v5 is ok once configured properly, fullstaq-ruby/server-edition#98 so we can easily construct those images ourselves and use LD_PRELOAD to inject the upgraded jemalloc system.

I prefer this as it frees us from relying on other folks building our base images and ensures we are flexible with the underlying image variations.

Review checklist

  • First, the most important one: is this PR small enough that you can actually review it? Feel free to just reject a branch if the changes are hard to review due to the length of the diff.
  • If there are any migrations, will they the previous version of the app work correctly after they've been run (e.g. the don't remove columns still known about by ActiveRecord).
  • If anything changed with regards to the public API, are those changes also documented in the apiary.apib file?
  • Are all the changes covered by tests? Think about any possible edge cases that might be left untested.

newer debian versions use v5 not v3 anymore, ruby 2.7 image is based on debian bullseye so we need to switch over to the new v5 jemalloc system
@camallen
Copy link
Contributor Author

this will need the CI system checks update to use the new 2.7 matrix ruby version

@camallen
Copy link
Contributor Author

this failure mode where our CI checks passed but our docker builds didn't work is another reason to re-implement a docker build check on each PR. We used to do this for each build when we used jenkins for CI, https://github.com/zooniverse/panoptes/blob/64c72ee8d5f2afabae1f9bd2ed125e734694f26d/Jenkinsfile#LL12

this would be a very valuable change to land in the GH Action CI setup.

Copy link
Collaborator

@yuenmichelle1 yuenmichelle1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. I'll go ahead and squash and merge. We probably want to do this for the Dockerfile-next and change the required action runs to Gemfile 2.7 labelled test runs .

@yuenmichelle1 yuenmichelle1 merged commit d14be7f into master Nov 22, 2022
@yuenmichelle1 yuenmichelle1 deleted the upgrade-ruby-fix-bundler branch November 22, 2022 16:08
@github-actions github-actions bot added the approved Approved pull request label Nov 22, 2022
@jjb
Copy link

jjb commented Mar 2, 2023

thanks for making this public, it was very helpful for me!

@jjb
Copy link

jjb commented Mar 7, 2023

i collected a bunch of jemalloc 5 info here: https://gist.github.com/jjb/9ff0d3f622c8bbe904fe7a82e35152fc

@camallen
Copy link
Contributor Author

camallen commented Mar 7, 2023

@jjb great work 👏🏼 🎉

@jjb
Copy link

jjb commented Mar 7, 2023

After this went live, did you have any observations about cpu and memory changes?

@camallen
Copy link
Contributor Author

camallen commented Mar 7, 2023

After this went live, did you have any observations about cpu and memory changes?

A slight increase in memory usage, not huge and cpu was similar and within normal bounds.

We’ve found jemalloc v5 to be work well with a slight increase in the RAM fragmentation vs v3 - however we’ve opted for the more performant variant trading CPU costs for some RAM.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Approved pull request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants